Thank you for opening this pull request! It may take us a few days to respond here, so thank you for being patient.
If you have questions, some answers may be found in our contributing guidelines.

that does look nicer. Another though I had was counting the number of NaNs; and call np.roll but not sure if that will work

Thanks @jsignell and @dcherian for the suggestions!

I tested @jsignell's approach locally - it's much cleaner and handles all null types via duck_array_ops.notnull (not just float NaN like my version). All existing sortby tests pass.

Happy to update the PR with this implementation and add a multidimensional test case, e.g.:

ds = Dataset({
    "var": (["x", "y"], [[1, 2], [3, 4], [5, 6]]),
    "key": ("x", [3.0, np.nan, 1.0])
})
result = ds.sortby("key", ascending=False)
# x order: [0, 2, 1] with NaN row at end

Happy to update the PR with this implementation and add a multidimensional test case

Maybe multidimensional wasn't quite what I meant actually. I would love to see a test where there are multiple arrays passed to the sortby. Something like this:

>>> ds = xr.Dataset(
...     {
...         "A": (("x", "y"), [[1, 2, 3], [4, 5, 6]]),
...         "B": (("x", "y"), [[7, 8, 9], [10, 11, 12]]),
...     },
...     coords={"x": ["b", "a"], "y": [np.nan, 1, 0]},
... )
>>> ds.sortby(["x", "y"], ascending=False)